Skip to content

ref(hub): Move @sentry/hub code to @sentry/core #5823

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 9 commits into from
Oct 7, 2022

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Sep 26, 2022

Closes #5665

This PR:

  • Moves all of the @sentry/hub code files to @sentry/core
  • Changes @sentry/hub to a stub that re-exports the same types from @sentry/core
    • @sentry/hub now only depends on @sentry/core
  • Switches internal imports @sentry/hub -> @sentry/core
    • Removes @sentry/hub as a dependency from packages
  • Fixes all the mocks/spys/imports in tests
  • Leaves the @sentry/hub tests where they are for now

Surprisingly this results in a 159 byte increase in minified bundle size!

I've compared the type exports from @sentry/hub from before and after this PR and the exports match so it looks like this isn't a breaking change 😬

@timfish timfish changed the title ref(hub): Move @sentry/hub code to @sentry/core ref(hub): Move @sentry/hub code to @sentry/core Sep 26, 2022
@timfish

This comment was marked as outdated.

@AbhiPrasad
Copy link
Member

We merged in #5873, which prob requires a more tricky rebase.

@timfish
Copy link
Collaborator Author

timfish commented Oct 5, 2022

Should we be marking all @sentry/hub exports as Deprecated - Moved to @sentry/core. @sentry/hub will be removed in v8?

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future readers: We'll save deleting @sentry/hub for the next major version, this just sets us up to do further refactors to work toward truly platform agnostic packages.

@AbhiPrasad
Copy link
Member

Should we be marking all @sentry/hub exports as Deprecated - Moved to @sentry/core. @sentry/hub will be removed in v8?

Good idea! Let's do that.

@timfish
Copy link
Collaborator Author

timfish commented Oct 5, 2022

Since the exports in @sentry/hub are just re-exports of external types, the only way to mark them as deprecated is to re-declare them as const and then mark with jsdoc like:

export { getCurrentHub as getCurrentHubCore } from '@sentry/core';

/**
 * @deprecated This export has moved to @sentry/core. The @sentry/hub package will be removed in v8 
 */
const getCurrentHub = getCurrentHubCore;

export { getCurrentHub }

@timfish
Copy link
Collaborator Author

timfish commented Oct 5, 2022

Ok, I think this is now good to go.

I couldn't mark Scope and Hub as deprecated like above because setting them as a const turns them into a value that can't then be used as a type!

@lobsterkatie
Copy link
Member

lobsterkatie commented Oct 7, 2022

I couldn't mark Scope and Hub as deprecated like above because setting them as a const turns them into a value that can't then be used as a type!

@timfish, since they're classes, could you do

import { Hub as CoreHub } from '@sentry/core';

/**
 * @deprecated This export has moved to @sentry/core. The @sentry/hub package will be removed in v8 
 */
export class Hub extends CoreHub { };

?

@timfish
Copy link
Collaborator Author

timfish commented Oct 7, 2022

It does leave me with a strange feeling that it is somehow exporting a different type to before but my logical side says that it's totally fine! 🙃

@AbhiPrasad AbhiPrasad merged commit 8570f0d into getsentry:master Oct 7, 2022
@AbhiPrasad
Copy link
Member

Can we move the tests in another PR?

Let's try to get this released on Monday alongside the NextJS experimental flag switch

@timfish
Copy link
Collaborator Author

timfish commented Oct 7, 2022

Leaving the tests in hub was instrumental in ensuring that there were no breaking changes in @sentry/hub exports.

Safe to move then now?

@AbhiPrasad
Copy link
Member

Leaving the tests in hub was instrumental in ensuring that there were no breaking changes in @sentry/hub exports.

Ahhh great point, then let's keep them until we do the actual deletion during the major.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v8] Remove @sentry/hub package
3 participants